Skip to content

Use SmallVec in NeighborTable for cache locality#10784

Merged
mtreinish merged 2 commits into
Qiskit:mainfrom
jakelishman:sabre/cache-local-neighbours
Sep 6, 2023
Merged

Use SmallVec in NeighborTable for cache locality#10784
mtreinish merged 2 commits into
Qiskit:mainfrom
jakelishman:sabre/cache-local-neighbours

Conversation

@jakelishman
Copy link
Copy Markdown
Member

Summary

A reasonable chunk of our time in Sabre is spent reading through the NeighborTable to find the candidate swaps for a given layout. Most coupling maps that we care about have a relatively low number of edges between qubits, yet we needed to redirect to the heap for each individual physical-qubit lookup currently.

This switches from using a Vec (which is always a fat pointer to heap memory) to SmallVec with an inline buffer space of four qubits. With the qubit type being u32, the SmallVec now takes up the same stack size as a Vec but can store (usually) all the swaps directly inline in the outer Vec of qubits. This means that most lookups of the available swaps are looking in the same (up to relatively small offsets) in memory, which makes the access patterns much easier for prefetching to optimise for.

Details and comments

This is a less pronounced effect than either #10782 or #10783, but is still a measurable performance improvement - the example in #10782 went from 2.08(4)s on main to 1.96(3)s with this PR (again - the numbers are bigger because Webex is using some of my CPU cycles haha).

A reasonable chunk of our time in Sabre is spent reading through the
`NeighborTable` to find the candidate swaps for a given layout.  Most
coupling maps that we care about have a relatively low number of edges
between qubits, yet we needed to redirect to the heap for each
individual physical-qubit lookup currently.

This switches from using a `Vec` (which is always a fat pointer to heap
memory) to `SmallVec` with an inline buffer space of four qubits.
With the qubit type being `u32`, the `SmallVec` now takes up the same
stack size as a `Vec` but can store (usually) all the swaps directly
inline in the outer `Vec` of qubits.  This means that most lookups of
the available swaps are looking in the same (up to relatively small
offsets) in memory, which makes the access patterns much easier for
prefetching to optimise for.
@jakelishman jakelishman added performance Changelog: None Do not include in the GitHub Release changelog. Rust This PR or issue is related to Rust code in the repository mod: transpiler Issues and PRs related to Transpiler labels Sep 6, 2023
@jakelishman jakelishman requested a review from a team as a code owner September 6, 2023 15:03
@qiskit-bot
Copy link
Copy Markdown
Collaborator

One or more of the the following people are requested to review this:

mtreinish
mtreinish previously approved these changes Sep 6, 2023
Copy link
Copy Markdown
Member

@mtreinish mtreinish left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM this is a very straighforward change and moving the data structure to small vec makes a lot of sense. I have one small question inline but it's non blocking more just curiosity than anything to change.

Comment thread crates/accelerate/src/sabre_swap/neighbor_table.rs Outdated
`SmallVec` doesn't have implementations of the PyO3 conversion trait, so
it needs to be done manually.  The previous state used to convert to a
Rust-space `Vec` that then needed to have its data moved from the Python
heap to the Rust heap.  This instead changes the conversions to interact
directly with Python lists, rather than using intermediary structures.
@mtreinish mtreinish added this pull request to the merge queue Sep 6, 2023
Merged via the queue into Qiskit:main with commit 180f19a Sep 6, 2023
@jakelishman jakelishman deleted the sabre/cache-local-neighbours branch September 6, 2023 22:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Changelog: None Do not include in the GitHub Release changelog. mod: transpiler Issues and PRs related to Transpiler performance Rust This PR or issue is related to Rust code in the repository

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants